Merge "Swap leading spaces to tabs in backup phpunit tests"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Sat, 26 Sep 2015 18:53:22 +0000 (18:53 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Sat, 26 Sep 2015 18:53:22 +0000 (18:53 +0000)
20 files changed:
RELEASE-NOTES-1.26
includes/User.php
includes/UserRightsProxy.php
includes/db/DBConnRef.php
includes/db/Database.php
includes/db/DatabaseMssql.php
includes/db/IDatabase.php
includes/filerepo/LocalRepo.php
includes/filerepo/file/LocalFile.php
includes/libs/BufferingStatsdDataFactory.php
includes/libs/CSSMin.php
includes/libs/objectcache/WANObjectCache.php
includes/parser/CacheTime.php
includes/parser/CoreParserFunctions.php
includes/parser/Parser.php
includes/parser/ParserCache.php
includes/parser/ParserOptions.php
includes/specialpage/QueryPage.php
includes/specials/SpecialMovepage.php
tests/phpunit/includes/LinkerTest.php

index aab1b42..7f738e1 100644 (file)
@@ -220,6 +220,8 @@ changes to languages because of Phabricator reports.
   'Project:Oversight' to 'Project:Suppress'.
 * (T84937) Free external links ("autolinked" urls) will now be terminated
   by &nbsp; and HTML entity encodings of &nbsp, <, and >.
+* DatabaseBase::resultObject() is now protected (use outside Database classes
+  not necessary since 1.11).
 
 == Compatibility ==
 
index d57dfaa..753061d 100644 (file)
@@ -2290,10 +2290,14 @@ class User implements IDBAccessObject {
         */
        public function clearSharedCache() {
                $id = $this->getId();
-               if ( $id ) {
-                       $key = wfMemcKey( 'user', 'id', $id );
-                       ObjectCache::getMainWANInstance()->delete( $key );
+               if ( !$id ) {
+                       return;
                }
+
+               $key = wfMemcKey( 'user', 'id', $id );
+               wfGetDB( DB_MASTER )->onTransactionPreCommitOrIdle( function() use ( $key ) {
+                       ObjectCache::getMainWANInstance()->delete( $key );
+               } );
        }
 
        /**
index a19f698..2c7032f 100644 (file)
@@ -278,8 +278,9 @@ class UserRightsProxy {
                        array( 'user_id' => $this->id ),
                        __METHOD__ );
 
-               $cache = ObjectCache::getMainWANInstance();
                $key = wfForeignMemcKey( $this->database, false, 'user', 'id', $this->id );
-               $cache->delete( $key );
+               $this->db->onTransactionPreCommitOrIdle( function() use ( $key ) {
+                       ObjectCache::getMainWANInstance()->delete( $key );
+               } );
        }
 }
index b4f3f79..ffada49 100644 (file)
@@ -449,10 +449,6 @@ class DBConnRef implements IDatabase {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
-       public function resultObject( $result ) {
-               return $this->__call( __FUNCTION__, func_get_args() );
-       }
-
        public function ping() {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
index 1e54f55..dbe86dc 100644 (file)
@@ -3766,14 +3766,13 @@ abstract class DatabaseBase implements IDatabase {
         * Once upon a time, DatabaseBase::query() returned a bare MySQL result
         * resource, and it was necessary to call this function to convert it to
         * a wrapper. Nowadays, raw database objects are never exposed to external
-        * callers, so this is unnecessary in external code. For compatibility with
-        * old code, ResultWrapper objects are passed through unaltered.
+        * callers, so this is unnecessary in external code.
         *
-        * @param bool|ResultWrapper|resource $result
+        * @param bool|ResultWrapper|resource|object $result
         * @return bool|ResultWrapper
         */
-       public function resultObject( $result ) {
-               if ( empty( $result ) ) {
+       protected function resultObject( $result ) {
+               if ( !$result ) {
                        return false;
                } elseif ( $result instanceof ResultWrapper ) {
                        return $result;
index 85f1b96..354afc5 100644 (file)
@@ -145,8 +145,8 @@ class DatabaseMssql extends DatabaseBase {
         * @param bool|MssqlResultWrapper|resource $result
         * @return bool|MssqlResultWrapper
         */
-       public function resultObject( $result ) {
-               if ( empty( $result ) ) {
+       protected function resultObject( $result ) {
+               if ( !$result ) {
                        return false;
                } elseif ( $result instanceof MssqlResultWrapper ) {
                        return $result;
index 49d0514..95608c6 100644 (file)
@@ -1347,22 +1347,6 @@ interface IDatabase {
         */
        public function timestampOrNull( $ts = null );
 
-       /**
-        * Take the result from a query, and wrap it in a ResultWrapper if
-        * necessary. Boolean values are passed through as is, to indicate success
-        * of write queries or failure.
-        *
-        * Once upon a time, DatabaseBase::query() returned a bare MySQL result
-        * resource, and it was necessary to call this function to convert it to
-        * a wrapper. Nowadays, raw database objects are never exposed to external
-        * callers, so this is unnecessary in external code. For compatibility with
-        * old code, ResultWrapper objects are passed through unaltered.
-        *
-        * @param bool|ResultWrapper|resource $result
-        * @return bool|ResultWrapper
-        */
-       public function resultObject( $result );
-
        /**
         * Ping the server and try to reconnect if it there is no connection
         *
index 6a2c064..b209bd6 100644 (file)
@@ -521,15 +521,11 @@ class LocalRepo extends FileRepo {
         * @return void
         */
        function invalidateImageRedirect( Title $title ) {
-               $cache = ObjectCache::getMainWANInstance();
-
-               $memcKey = $this->getSharedCacheKey( 'image_redirect', md5( $title->getDBkey() ) );
-               if ( $memcKey ) {
-                       // Set a temporary value for the cache key, to ensure
-                       // that this value stays purged long enough so that
-                       // it isn't refreshed with a stale value due to a
-                       // lagged slave.
-                       $cache->delete( $memcKey, 12 );
+               $key = $this->getSharedCacheKey( 'image_redirect', md5( $title->getDBkey() ) );
+               if ( $key ) {
+                       $this->getMasterDB()->onTransactionPreCommitOrIdle( function() use ( $key ) {
+                               ObjectCache::getMainWANInstance()->delete( $key );
+                       } );
                }
        }
 
index 4c9c2aa..3225d78 100644 (file)
@@ -321,7 +321,9 @@ class LocalFile extends File {
                        return;
                }
 
-               ObjectCache::getMainWANInstance()->delete( $key );
+               $this->repo->getMasterDB()->onTransactionPreCommitOrIdle( function() use ( $key ) {
+                       ObjectCache::getMainWANInstance()->delete( $key );
+               } );
        }
 
        /**
index 100d2a4..a0020da 100644 (file)
@@ -57,7 +57,9 @@ class BufferingStatsdDataFactory extends StatsdDataFactory {
                return str_replace( array( '._', '_.' ), '.', $key );
        }
 
-       public function produceStatsdData( $key, $value = 1, $metric = StatsdDataInterface::STATSD_METRIC_COUNT ) {
+       public function produceStatsdData(
+               $key, $value = 1, $metric = StatsdDataInterface::STATSD_METRIC_COUNT
+       ) {
                $entity = $this->produceStatsdDataEntity();
                if ( $key !== null ) {
                        $key = self::normalizeMetricKey( "{$this->prefix}.{$key}" );
index c93206c..6ca0fed 100644 (file)
@@ -304,7 +304,17 @@ class CSSMin {
                                // Check for global @embed comment and remove it. Allow other comments to be present
                                // before @embed (they have been replaced with placeholders at this point).
                                $embedAll = false;
-                               $rule = preg_replace( '/^((?:\s+|' . CSSMin::PLACEHOLDER . '(\d+)x)*)' . CSSMin::EMBED_REGEX . '\s*/', '$1', $rule, 1, $embedAll );
+                               $rule = preg_replace(
+                                       '/^((?:\s+|' .
+                                               CSSMin::PLACEHOLDER .
+                                               '(\d+)x)*)' .
+                                               CSSMin::EMBED_REGEX .
+                                               '\s*/',
+                                       '$1',
+                                       $rule,
+                                       1,
+                                       $embedAll
+                               );
 
                                // Build two versions of current rule: with remapped URLs
                                // and with embedded data: URIs (where possible).
index fb96269..c78b299 100644 (file)
@@ -286,6 +286,31 @@ class WANObjectCache {
         *   - a) Replication lag is bounded to being less than HOLDOFF_TTL; or
         *   - b) If lag is higher, the DB will have gone into read-only mode already
         *
+        * When using potentially long-running ACID transactions, a good pattern is
+        * to use a pre-commit hook to issue the delete. This means that immediately
+        * after commit, callers will see the tombstone in cache in the local datacenter
+        * and in the others upon relay. It also avoids the following race condition:
+        *   - a) T1 begins, changes a row, and calls delete()
+        *   - b) The HOLDOFF_TTL passes, expiring the delete() tombstone
+        *   - c) T2 starts, reads the row and calls set() due to a cache miss
+        *   - d) T1 finally commits
+        *   - e) Stale value is stuck in cache
+        *
+        * Example usage:
+        * @code
+        *     $dbw->begin(); // start of request
+        *     ... <execute some stuff> ...
+        *     // Update the row in the DB
+        *     $dbw->update( ... );
+        *     $key = wfMemcKey( 'homes', $homeId );
+        *     // Purge the corresponding cache entry just before committing
+        *     $dbw->onTransactionPreCommitOrIdle( function() use ( $cache, $key ) {
+        *         $cache->delete( $key );
+        *     } );
+        *     ... <execute some stuff> ...
+        *     $dbw->commit(); // end of request
+        * @endcode
+        *
         * If called twice on the same key, then the last hold-off TTL takes
         * precedence. For idempotence, the $ttl should not vary for different
         * delete() calls on the same key. Also note that lowering $ttl reduces
index c450689..7acfe38 100644 (file)
@@ -32,10 +32,17 @@ class CacheTime {
         */
        public $mUsedOptions;
 
-       public $mVersion = Parser::VERSION,  # Compatibility check
-               $mCacheTime = '',             # Time when this object was generated, or -1 for uncacheable. Used in ParserCache.
-               $mCacheExpiry = null,         # Seconds after which the object should expire, use 0 for uncacheable. Used in ParserCache.
-               $mCacheRevisionId = null;     # Revision ID that was parsed
+       # Compatibility check
+       public $mVersion = Parser::VERSION;
+
+       # Time when this object was generated, or -1 for uncacheable. Used in ParserCache.
+       public $mCacheTime = '';
+
+       # Seconds after which the object should expire, use 0 for uncacheable. Used in ParserCache.
+       public $mCacheExpiry = null;
+
+       # Revision ID that was parsed
+       public $mCacheRevisionId = null;
 
        /**
         * @return string TS_MW timestamp
index 7639e2f..b0737f6 100644 (file)
@@ -60,7 +60,11 @@ class CoreParserFunctions {
                        $parser->setFunctionHook( $func, array( __CLASS__, $func ), Parser::SFH_NO_HASH );
                }
 
-               $parser->setFunctionHook( 'namespace', array( __CLASS__, 'mwnamespace' ), Parser::SFH_NO_HASH );
+               $parser->setFunctionHook(
+                       'namespace',
+                       array( __CLASS__, 'mwnamespace' ),
+                       Parser::SFH_NO_HASH
+               );
                $parser->setFunctionHook( 'int', array( __CLASS__, 'intFunction' ), Parser::SFH_NO_HASH );
                $parser->setFunctionHook( 'special', array( __CLASS__, 'special' ) );
                $parser->setFunctionHook( 'speciale', array( __CLASS__, 'speciale' ) );
@@ -68,7 +72,11 @@ class CoreParserFunctions {
                $parser->setFunctionHook( 'formatdate', array( __CLASS__, 'formatDate' ) );
 
                if ( $wgAllowDisplayTitle ) {
-                       $parser->setFunctionHook( 'displaytitle', array( __CLASS__, 'displaytitle' ), Parser::SFH_NO_HASH );
+                       $parser->setFunctionHook(
+                               'displaytitle',
+                               array( __CLASS__, 'displaytitle' ),
+                               Parser::SFH_NO_HASH
+                       );
                }
                if ( $wgAllowSlowParserFunctions ) {
                        $parser->setFunctionHook(
@@ -810,7 +818,9 @@ class CoreParserFunctions {
         * @param int $direction
         * @return string
         */
-       public static function pad( $parser, $string, $length, $padding = '0', $direction = STR_PAD_RIGHT ) {
+       public static function pad(
+               $parser, $string, $length, $padding = '0', $direction = STR_PAD_RIGHT
+       ) {
                $padding = $parser->killMarkers( $padding );
                $lengthOfPadding = mb_strlen( $padding );
                if ( $lengthOfPadding == 0 ) {
index 3e5e8a1..3b49ccf 100644 (file)
@@ -511,7 +511,10 @@ class Parser {
                        }
                        $limitReport .= 'Cached time: ' . $this->mOutput->getCacheTime() . "\n";
                        $limitReport .= 'Cache expiry: ' . $this->mOutput->getCacheExpiry() . "\n";
-                       $limitReport .= 'Dynamic content: ' . ( $this->mOutput->hasDynamicContent() ? 'true' : 'false' ) . "\n";
+                       $limitReport .= 'Dynamic content: ' .
+                               ( $this->mOutput->hasDynamicContent() ? 'true' : 'false' ) .
+                               "\n";
+
                        foreach ( $this->mOutput->getLimitReportData() as $key => $value ) {
                                if ( Hooks::run( 'ParserLimitReportFormat',
                                        array( $key, &$value, &$limitReport, false, false )
@@ -1468,7 +1471,6 @@ class Parser {
         * @private
         */
        public function makeFreeExternalLink( $url, $numPostProto ) {
-
                $trail = '';
 
                # The characters '<' and '>' (which were escaped by
@@ -1476,7 +1478,12 @@ class Parser {
                # URLs, per RFC 2396.
                # Make &nbsp; terminate a URL as well (bug T84937)
                $m2 = array();
-               if ( preg_match( '/&(lt|gt|nbsp|#x0*(3[CcEe]|[Aa]0)|#0*(60|62|160));/', $url, $m2, PREG_OFFSET_CAPTURE ) ) {
+               if ( preg_match(
+                       '/&(lt|gt|nbsp|#x0*(3[CcEe]|[Aa]0)|#0*(60|62|160));/',
+                       $url,
+                       $m2,
+                       PREG_OFFSET_CAPTURE
+               ) ) {
                        $trail = substr( $url, $m2[0][1] ) . $trail;
                        $url = substr( $url, 0, $m2[0][1] );
                }
index abff543..c03b5c2 100644 (file)
@@ -226,11 +226,17 @@ class ParserCache {
                        wfIncrStats( "pcache.miss.revid" );
                        $revId = $article->getLatest();
                        $cachedRevId = $value->getCacheRevisionId();
-                       wfDebug( "ParserOutput key is for an old revision, latest $revId, cached $cachedRevId\n" );
+                       wfDebug(
+                               "ParserOutput key is for an old revision, latest $revId, cached $cachedRevId\n"
+                       );
                        $value = false;
-               } elseif ( Hooks::run( 'RejectParserCacheValue', array( $value, $wikiPage, $popts ) ) === false ) {
+               } elseif (
+                       Hooks::run( 'RejectParserCacheValue', array( $value, $wikiPage, $popts ) ) === false
+               ) {
                        wfIncrStats( 'pcache.miss.rejected' );
-                       wfDebug( "ParserOutput key valid, but rejected by RejectParserCacheValue hook handler.\n" );
+                       wfDebug(
+                               "ParserOutput key valid, but rejected by RejectParserCacheValue hook handler.\n"
+                       );
                        $value = false;
                } else {
                        wfIncrStats( "pcache.hit" );
@@ -284,7 +290,10 @@ class ParserCache {
                        // ...and its pointer
                        $this->mMemc->set( $this->getOptionsKey( $page ), $optionsKey, $expire );
 
-                       Hooks::run( 'ParserCacheSaveComplete', array( $this, $parserOutput, $page->getTitle(), $popts, $revId ) );
+                       Hooks::run(
+                               'ParserCacheSaveComplete',
+                               array( $this, $parserOutput, $page->getTitle(), $popts, $revId )
+                       );
                } else {
                        wfDebug( "Parser output was marked as uncacheable and has not been saved.\n" );
                }
index 1073aed..e6d5274 100644 (file)
@@ -831,8 +831,8 @@ class ParserOptions {
        }
 
        /**
-        * Sets a hook to force that a page exists, and sets a current revision callback to return a
-        * revision with custom content when the current revision of the page is requested.
+        * Sets a hook to force that a page exists, and sets a current revision callback to return
+        * revision with custom content when the current revision of the page is requested.
         *
         * @since 1.25
         * @param Title $title
@@ -841,20 +841,25 @@ class ParserOptions {
         * @return ScopedCallback to unset the hook
         */
        public function setupFakeRevision( $title, $content, $user ) {
-               $oldCallback = $this->setCurrentRevisionCallback( function ( $titleToCheck, $parser = false ) use ( $title, $content, $user, &$oldCallback ) {
-                       if ( $titleToCheck->equals( $title ) ) {
-                               return new Revision( array(
-                                       'page' => $title->getArticleID(),
-                                       'user_text' => $user->getName(),
-                                       'user' => $user->getId(),
-                                       'parent_id' => $title->getLatestRevId(),
-                                       'title' => $title,
-                                       'content' => $content
-                               ) );
-                       } else {
-                               return call_user_func( $oldCallback, $titleToCheck, $parser );
+               $oldCallback = $this->setCurrentRevisionCallback(
+                       function (
+                               $titleToCheck, $parser = false ) use ( $title, $content, $user, &$oldCallback
+                       ) {
+                               if ( $titleToCheck->equals( $title ) ) {
+                                       return new Revision( array(
+                                               'page' => $title->getArticleID(),
+                                               'user_text' => $user->getName(),
+                                               'user' => $user->getId(),
+                                               'parent_id' => $title->getLatestRevId(),
+                                               'title' => $title,
+                                               'content' => $content
+                                       ) );
+                               } else {
+                                       return call_user_func( $oldCallback, $titleToCheck, $parser );
+                               }
                        }
-               } );
+               );
+
                global $wgHooks;
                $wgHooks['TitleExists'][] =
                        function ( $titleToCheck, &$exists ) use ( $title ) {
index 3c8b742..bfb29ae 100644 (file)
@@ -450,14 +450,13 @@ abstract class QueryPage extends SpecialPage {
                } else {
                        $options['ORDER BY'] = 'qc_value ASC';
                }
-               $res = $dbr->select( 'querycache', array( 'qc_type',
+               return $dbr->select( 'querycache', array( 'qc_type',
                                'namespace' => 'qc_namespace',
                                'title' => 'qc_title',
                                'value' => 'qc_value' ),
                                array( 'qc_type' => $this->getName() ),
                                __METHOD__, $options
                );
-               return $dbr->resultObject( $res );
        }
 
        public function getCachedTimestamp() {
index f7a0a20..ab28fa4 100644 (file)
@@ -464,7 +464,7 @@ class MovePageForm extends UnlistedSpecialPage {
                                'name' => $submitVar,
                                'value' => $movepagebtn,
                                'label' => $movepagebtn,
-                               'flags' => array( 'progressive', 'primary' ),
+                               'flags' => array( 'constructive', 'primary' ),
                                'type' => 'submit',
                        ) ),
                        array(
index a3efbb8..14d0762 100644 (file)
@@ -93,7 +93,9 @@ class LinkerTest extends MediaWikiLangTestCase {
         * @covers Linker::formatAutocomments
         * @covers Linker::formatLinksInComment
         */
-       public function testFormatComment( $expected, $comment, $title = false, $local = false, $wikiId = null ) {
+       public function testFormatComment(
+               $expected, $comment, $title = false, $local = false, $wikiId = null
+       ) {
                $conf = new SiteConfiguration();
                $conf->settings = array(
                        'wgServer' => array(